Pod logs operation #50
Conversation
add pod log operation support
|
Oh, this is really good! Thanks a lot for doing this. Looks like you ran into quite a few special cases. Will try to review this one a bit, as there are a few things I'm a bit unsure about, but overall it looks good. |
examples/log_openapi.rs
Outdated
| let config = config::load_kube_config().expect("failed to load kubeconfig"); | ||
| let client = APIClient::new(config); | ||
|
|
||
| let mypod: &str = "mypod"; |
| @@ -58,6 +59,8 @@ impl Api<Object<PodSpec, PodStatus>> { | |||
| } | |||
| } | |||
|
|
|||
| impl Log for Api<Object<PodSpec, PodStatus>> {} | |||
There was a problem hiding this comment.
Rather than having a one line impl for one resource object that every user would need to import the trait of, could we instead:
- have
logalways public butunimplemented!()or{}by default - have an extra
impl Api<Object<PodSpec, PodStatus>>that overrideslog
Then you have one less import required for users.
There was a problem hiding this comment.
I implements log operation in this way, because I hope trait Log can be reused by other developers when they implement their crds.
There was a problem hiding this comment.
Yeah, that makes sense. Either way is fine, it would just be easier for users of the Api to not have to import the extra trait.
| fn log(&self, name: &str, lp: &LogParams) -> Result<String> { | ||
| Ok(self.log_operation(name, lp)?) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a nice setup between private and public methods that looks like it allows you to very easily implement the Log trait. Cool pattern. Unfortunately, if you follow do my suggestion, this won't be needed.
|
I'll merge this as it stands, my other point is not important. Thanks again! |
using traits to implements log operation in #30